Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ListOffsetsRequest should only be sent to the leader replica (CI) #4754

Merged
merged 4 commits into from
Oct 18, 2024

Conversation

emasab
Copy link
Contributor

@emasab emasab commented Jun 12, 2024

Continuation of #4616 from an internal branch due to CI issues

@emasab emasab requested a review from a team as a code owner June 12, 2024 12:05
@emasab emasab changed the title ListOffsetsRequest should only be sent to the leader replica ListOffsetsRequest should only be sent to the leader replica (CI) Jun 12, 2024
@mjd95
Copy link

mjd95 commented Jul 18, 2024

@emasab do you think this PR could be merged soon? I saw there were some CI issues on previous iterations of this PR but this one looks green

Copy link
Contributor

@milindl milindl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kphelps and @emasab . Looks good to me bar a few minor documentation related things.

@@ -111,6 +110,11 @@ static void do_test_offset_reset(const char *auto_offset_reset) {
else
test_consumer_poll(auto_offset_reset, c, 0, 1, 0, msgcnt, NULL);

/* send another batch of messages to ensure the consumer isn't stuck */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea to expand on why exactly we add this and why the consumer was getting stuck, alternatively, we can provide the issue # . Without that, it'll be a bit difficult for future readers.

CHANGELOG.md Outdated
* Fix segfault when using long client id because of erased segment when using flexver. (#4689)
* Update bundled lz4 (#4726)
* Fix segfault when using long client id because of erased segment when using flexver. (#4689)
* Fix for a loop of ListOffset requests, happening in a Fetch From Follower
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix changelog for this to be in 2.5.1

@confluent-cla-assistant
Copy link

confluent-cla-assistant bot commented Sep 24, 2024

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@airlock-confluentinc airlock-confluentinc bot force-pushed the dev_kphelps_list-offsets-leader branch 2 times, most recently from 4d463c9 to 89369d8 Compare October 17, 2024 15:07
@emasab emasab merged commit 2810e5c into master Oct 18, 2024
2 checks passed
@emasab emasab deleted the dev_kphelps_list-offsets-leader branch October 18, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants